Skip to content

Avoid focus loss on switching tabs #1859

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 9, 2025
Merged

Avoid focus loss on switching tabs #1859

merged 1 commit into from
Oct 9, 2025

Conversation

katinthehatsite
Copy link
Contributor

Related issues

Proposed Changes

This PR ensures that we don't lose focus when we use the keyboard navigation when switching between tabs.

Testing Instructions

  • Pull the changes from this branch
  • Start Studio
  • Click on one of the tabs e.g. Overview
  • Try using left and right arrows to move through tabs
  • Confirm that you can move between tabs and that they do not lose focus
  • Compare this behaviour to trunk and observe that keyboard navigation does not work correctly there and you can't move through tabs

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@katinthehatsite katinthehatsite self-assigned this Oct 8, 2025
@katinthehatsite katinthehatsite marked this pull request as draft October 8, 2025 14:21
@katinthehatsite katinthehatsite marked this pull request as ready for review October 9, 2025 07:51
@katinthehatsite katinthehatsite requested a review from a team October 9, 2025 07:51
bcotrim
bcotrim reviewed Oct 9, 2025
View reviewed changes
Copy link
Contributor

@bcotrim bcotrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered changing the TabPanel key to selectedSite.id?
I think it would also fix the issue without needing the isFirstRender lastChangeWasUser logic.

Unrelated observation: While testing keyboard navigation, I noticed that navigating to the AI Assistant tab moves focus to the input. This is pre-existing behavior from trunk, but it feels unexpected when navigating via keyboard. Might be worth a separate issue?

@katinthehatsite
Copy link
Contributor Author

Have you considered changing the TabPanel key to selectedSite.id?
I think it would also fix the issue without needing the isFirstRender lastChangeWasUser logic.

I did and this approach would be simpler but would it not affect setting the tabs like this, for example?

studio/src/hooks/use-site-details.tsx

Line 200 in 3348f90

setSelectedTab( 'overview' );

@bcotrim
Copy link
Contributor

bcotrim commented Oct 9, 2025

I did and this approach would be simpler but would it not affect setting the tabs like this, for example?

You are right, on deleteSiteit would work since it also changes selected site ID, but on useListenDeepLinkConnection it might break. Let's keep your original solution, thanks for the clarification!

gcsecsey
gcsecsey approved these changes Oct 9, 2025
View reviewed changes
Copy link
Contributor

@gcsecsey gcsecsey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes LGTM. I was also thinking about potential simplifications, thanks for the explanation above Kat for why the useEffect and the refs are needed.

Functionally, this works great:

CleanShot.2025-10-09.at.12.29.07.mp4

@katinthehatsite
Copy link
Contributor Author

The changes LGTM. I was also thinking about potential simplifications, thanks for the explanation above Kat for why the useEffect and the refs are needed.

Yeah I spent some time dancing around it yesterday but could not find something simpler to make sure there is no regressions 😅

@katinthehatsite katinthehatsite merged commit d379ff0 into trunk Oct 9, 2025
12 checks passed
@katinthehatsite katinthehatsite deleted the fix/tabs-resetting-screen-reader-focus branch October 9, 2025 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bcotrim bcotrim bcotrim approved these changes

@gcsecsey gcsecsey gcsecsey approved these changes

Assignees

@katinthehatsite katinthehatsite

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@katinthehatsite @bcotrim @gcsecsey